feat: add post accounts in simulation#1099
Conversation
📝 WalkthroughWalkthroughThe PR adds returning post-simulation account state to transaction simulations. It extends TransactionSimulationResult with a post_simulation_accounts field, populates that field in the processor's simulate path (including executed transactions), and updates the HTTP simulate handler to resolve, validate, encode, and return requested accounts (merging post-simulation over current accounts). A new test asserts that requested accounts are returned with expected balances and entries. Assessment against linked issues
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Manual Deploy AvailableYou can trigger a manual deploy of this PR branch to testnet: Alternative: Comment
Comment updated automatically when the PR is synchronized. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4128974682
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d18a9da12a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@magicblock-aperture/src/requests/http/simulate_transaction.rs`:
- Around line 77-82: The error message returned when rejecting unsupported
encodings is inaccurate: the conditional checks for UiAccountEncoding::Binary or
UiAccountEncoding::Base58 but the RpcError::invalid_params message only mentions
"base58 encoding not supported"; update the error text in the
RpcError::invalid_params call (in the conditional that inspects
accounts_encoding) to reflect both unsupported encodings (e.g., "binary and
base58 encodings not supported" or similar) so the message matches the checked
UiAccountEncoding variants.
In `@magicblock-processor/src/executor/processing.rs`:
- Around line 120-139: The current branch handling
ProcessedTransaction::Executed only collects
executed.loaded_transaction.accounts up to number_of_accounts, which omits any
additionally requested pubkeys and forces callers to patch accounts later;
update the code that builds the simulation request to include the requested
pubkeys (e.g., a field like simulation_request.requested_pubkeys) and here in
the match arm for ProcessedTransaction::Executed materialize those requested
accounts from the same simulated snapshot instead of only using
executed.loaded_transaction.accounts — replace the post_simulation_accounts =
executed.loaded_transaction.accounts.into_iter().take(number_of_accounts).collect()
logic with code that merges/constructs accounts by pulling each requested pubkey
from the snapshot (using the same snapshot source used by the simulator) so the
returned accounts array contains the post-simulation state for all requested
addresses.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 20290f3d-56f4-49bb-9203-9682e60705fd
📒 Files selected for processing (4)
magicblock-aperture/src/requests/http/simulate_transaction.rsmagicblock-aperture/tests/transactions.rsmagicblock-core/src/link/transactions.rsmagicblock-processor/src/executor/processing.rs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 638b33ed6e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
bmuddha
left a comment
There was a problem hiding this comment.
LGTM, with couple minor nits
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7112f30cc8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@magicblock-aperture/src/requests/http/simulate_transaction.rs`:
- Around line 55-125: Validate config.accounts (encoding, address count, and
parse each address into Pubkey) before calling
self.transactions_scheduler.simulate(...) so malformed params or unsupported
encodings are rejected prior to consuming scheduler work; then after simulation
use the returned TransactionSimulationResult.post_simulation_accounts map to
assemble the response for each requested pubkey (mapping missing entries to
None) instead of calling read_accounts_with_ensure (remove the fallback to
current AccountsDb), and preserve the existing behavior of returning
Some(vec![None; ...]) only when result_err.is_some() after validation and
parsing have already occurred.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a6eb3e83-fbc3-4028-ad51-481250e169e4
📒 Files selected for processing (2)
magicblock-aperture/src/requests/http/simulate_transaction.rsmagicblock-core/src/link/transactions.rs
Summary
Compatibility
Testing
Checklist
Summary by CodeRabbit